Skip to content

refactor: fix correctness bugs, add tests, and document the package#1

Merged
vyncint merged 2 commits into
mainfrom
refactor/professional-cleanup
Jul 25, 2026
Merged

refactor: fix correctness bugs, add tests, and document the package#1
vyncint merged 2 commits into
mainfrom
refactor/professional-cleanup

Conversation

@vyncint

@vyncint vyncint commented Jul 25, 2026

Copy link
Copy Markdown
Owner

Review, refactor and documentation pass over the package. Backward compatible with 2.0.0 — existing code keeps working and warns where a name has changed. Ships as 2.1.0.

Why

The happy path worked, but a few edge cases were broken, tests only ran when a release was published, and the README stopped after one example. This addresses all three.

Correctness fixes

Problem Effect Fix
target_value or <bound> Target(target_value=0) silently became an unreachable bound, so training could never stop on that target Test for None
last_value only assigned inside the training loop AttributeError before the first round, after an interrupt during it, and on every resumed session Derived from the persisted history
target: Target = Target() One mutable default shared by every trainer instance None sentinel
No guard before compile() AttributeError on an internal attribute RuntimeError naming the missing call
Absent metric name Bare KeyError RuntimeError listing available keys, hinting at validation_data for val_*
Parent dirs not created Nested checkpoint paths failed Created on save
Values restored as 0-d object arrays Odd types leaking into user code Converted to float / plain weight lists
requires-python = ">=3.9" TensorFlow has required >=3.10 since 2.16, so the floor advertised an uninstallable combination Corrected to >=3.10
polars dependency Never imported Removed

Structure

  • Split into target.py, trainer.py, _storage.py. Public imports unchanged.
  • InfinityTrainingInfiniteTrainer, optimize_*best_*. Old names, attributes, constructor keywords and predict_optimize() all still work and emit DeprecationWarning until 3.0.0 — pinned by tests/test_deprecations.py.
  • run.pyexamples/mnist.py, so it no longer ships inside the wheel.
  • Stop reasons reported through logging; Ctrl+C is no longer swallowed silently.

CI

Tests previously ran only on release publication, so nothing gated a commit. Added ci.yml: ruff lint + format, pytest on Python 3.10/3.11/3.12, and a build + twine check, on every push and PR. The publish workflow now also verifies the release tag matches the packaged version.

Tests

55, up from 3 — targets and comparison rules, stop conditions, best-weight tracking, persistence and resume across sessions, error handling, and the deprecation shims.

Docs

README rewritten with a quick start, a "how it works" walkthrough, a resume guide, a full API reference, a timeout-granularity caveat and a security note about pickled checkpoints. Added CHANGELOG.md and CONTRIBUTING.md.

Verification

pytest tests            55 passed
ruff check .            All checks passed
ruff format --check .   13 files already formatted
python -m build         built sdist + wheel
twine check dist/*      PASSED

The wheel contains only infinite_training/ — no tests, no examples. The README quick start and resume snippets were run verbatim: the quick start reaches its 1e-4 target after 25 rounds, and a fresh trainer on the same paths reports rounds_completed == 25 with last_value and best_value restored.

Notes for review

  • The two behaviour fixes users could notice are the target_value=0 change and last_value returning None instead of raising; both are in the changelog.
  • requires-python now excludes 3.9. Worth a sanity check that you are happy dropping a version that could not have had a working TensorFlow anyway.

The package worked for the happy path but had several defects that only
appeared at the edges, no CI on commits, and a README that stopped at a
single example. This reworks all three while keeping the 2.0.x API
working.

Correctness fixes:

- Target(target_value=0) was discarded. The default was chosen with
  `target_value or <bound>`, so a falsy 0 became an unreachable bound and
  the session could never stop on that target.
- last_value raised AttributeError before the first round, and after an
  interrupt during it. It is now derived from the persisted history, so it
  is also correct straight after a restart.
- Target() was a mutable default argument shared by every trainer.
- train()/predict before compile() raised AttributeError on an internal
  attribute; they now raise RuntimeError explaining what to call.
- A target naming an absent metric raised a bare KeyError; it now lists the
  available keys and points at validation_data for val_* names.
- Checkpoint parent directories were not created, so nested paths failed.
- Restored values stayed as 0-d object arrays instead of floats/lists.
- requires-python said >=3.9, but TensorFlow has needed >=3.10 since 2.16.
- Dropped the unused polars dependency.

Structure and tooling:

- Split into target.py, trainer.py and _storage.py; public imports unchanged.
- Renamed InfinityTraining -> InfiniteTrainer and optimize_* -> best_*, with
  the old names kept as aliases that emit DeprecationWarning until 3.0.0.
- Added a CI workflow running lint, tests on 3.10-3.12 and a packaging check
  on every push and PR. Tests previously ran only when a release was cut.
- The publish workflow now verifies the tag matches the packaged version.
- Moved run.py to examples/mnist.py so it no longer ships in the wheel.

Tests and docs:

- 55 tests, up from 3, covering targets, stop conditions, persistence and
  resume, error handling, and the deprecation shims.
- Rewrote the README with an API reference, a resume guide, a timeout
  caveat and a checkpoint security note; added CHANGELOG.md and
  CONTRIBUTING.md.

Verified: 55 passed, ruff check and format clean, twine check passed, and
the README quick-start and resume flows run as written.
Copilot AI review requested due to automatic review settings July 25, 2026 09:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Raising target-version to py310 enabled B905, and zip() over weight lists
is exactly where an unnoticed length mismatch would matter, so pass
strict=True rather than silencing it. Also import Callable from
collections.abc.
@vyncint
vyncint merged commit 103f17f into main Jul 25, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants